Support multi-layer algorithms that receive a data product from an unfold#571
Open
knoepfel wants to merge 4 commits into
Open
Support multi-layer algorithms that receive a data product from an unfold#571knoepfel wants to merge 4 commits into
knoepfel wants to merge 4 commits into
Conversation
knoepfel
force-pushed
the
multi-layer-failure
branch
from
381d7c1 to
fa25cb1
Compare
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## main #571 +/- ##
==========================================
+ Coverage 82.11% 82.61% +0.50%
==========================================
Files 157 161 +4
Lines 5720 5891 +171
Branches 646 686 +40
==========================================
+ Hits 4697 4867 +170
+ Misses 808 807 -1
- Partials 215 217 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 4 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
knoepfel
force-pushed
the
multi-layer-failure
branch
12 times, most recently
from
eb2cf70 to
f4599e5
Compare
knoepfel
force-pushed
the
multi-layer-failure
branch
from
f4599e5 to
8cac667
Compare
…uct from an unfold
knoepfel
force-pushed
the
multi-layer-failure
branch
6 times, most recently
from
3bcd7a6 to
1e0b7a6
Compare
knoepfel
force-pushed
the
multi-layer-failure
branch
from
1e0b7a6 to
143de65
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes a bug where transforms operating on a hierarchy layer downstream of an unfold could not receive data products produced by that unfold. The root cause was that the
index_routerhad no mechanism to learn — from the unfold itself — how many children it had generated, so it could not emit the correct flush token at the right time.Solution
The fix introduces two new components and refactors the flush/routing pipeline.
New:
data_cell_tracker(phlex/model/)Tracks the sequence of incoming
data_cell_indexvalues from the driver and determines which flush tokens are ready to be emitted when a new index arrives or the job ends. This logic was previously implicit in the source/index-router interaction.New:
child_tracker(phlex/model/)Per-index tracker that tracks how many children have been processed across each child layer. Once the expected count (received from the unfold's flush result) is satisfied,
child_trackerfires a callback to emit the flush token for that index.declared_unfoldNow uses a third TBB output port (
index_flush) carrying the child counts alongside the existing message and index-message ports.index_routerGains a
flush_receiverinput and anestablish_layers()initializer that records which layers are produced by unfolds vs. consumed as inputs.route()now accepts the closeout flushes fromdata_cell_trackerrather than computing them internally, anddrain()likewise receives the remaining flushes at job end.framework_graphindex_receiver_node that decouples closeout flush emission from the source node.data_cell_tracker(cell_tracker_) and feeds its output intoindex_router_.route().index_router_.establish_layers()beforefinalize(), using layer metadata gathered from the declared unfolds.flush_sender()toindex_router_.flush_receiver().edge_makerRefactored to return
(provider_input_ports, multilayer_join_index_ports)instead of directly callingindex_router_.finalize(), allowingframework_graphto callestablish_layers()first.Tests
test/data_cell_tracker_test.cpp— unit tests fordata_cell_trackerin isolation.test/child_tracker_test.cpp— unit-level tests forchild_trackerverifying that committed child counts accumulate correctly through nested unfolds without involving the fullframework_graphmachinery.test/fold_duplicate_layer_name_test.cpp— re-enables the previously disableddifferent_hierarchiestest, now covering the case where two layers share the same name in a fold scenario.test/unfold.cpp— additional unfold scenarios exercising multi-layer product consumption.Files changed
phlex/model/data_cell_tracker.hpp,phlex/model/data_cell_tracker.cpp,phlex/model/child_tracker.hpp,phlex/model/child_tracker.cppphlex/core/index_router.hpp,phlex/core/index_router.cpp,phlex/core/framework_graph.hpp,phlex/core/framework_graph.cpp,phlex/core/declared_unfold.hpp,phlex/core/edge_maker.hpp,phlex/model/fixed_hierarchy.hpp,phlex/model/fixed_hierarchy.cpptest/data_cell_tracker_test.cpp,test/child_tracker_test.cpp,test/fold_duplicate_layer_name_test.cpptest/unfold.cppSummary largely courtesy of Claude Sonnet 4.6
Resolves #359